feat: retry and redrive dead effects and broadcasts - #78
Merged
Merged
Conversation
A dead effect or broadcast had no retry API. The only way back was an
operator writing an UPDATE against a runtime table, which is the one
thing the deny-by-default posture exists to prevent. A dead transmit
effect was a lost replay.
`SolidObjects.dead_letters` keeps its message meaning and answers two
scopes, so the kind rides on the receiver rather than on an argument:
SolidObjects.dead_letters.effects.all(authorization_context:)
SolidObjects.dead_letters.effects.retry(effect_id, ...)
SolidObjects.dead_letters.broadcasts.retry(broadcast_id, ...)
`retry` returns a dead row to pending with a zero attempt count, no
claim, and an immediate availability. It reuses the stable id, so a
deduplicating handler still sees the same key, and it acts only on a
dead row, so calling it twice cannot double-enqueue and cannot yank a
row a worker holds.
Each scope authorizes under its own resource name, and reads only its
own kind.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dashboard kept no record of an administration action, and retry is the action that can re-run a side effect. Every retry now writes one row to `solid_objects_administration_events`, holding the action, the kind, the subject, the identity, and when it happened. The identity comes from the existing authorization context through a new `administration_identity` hook, so the library records what the application already knows rather than invent an authentication concept. It defaults to the context's own `to_s` and is bounded to 255 bytes. A refused caller writes nothing, because the audit records what happened rather than what was attempted and denied. A read writes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retry was one row at a time, and an incident produces dead rows in the
hundreds. A scope answers `redrive` now, which opens a durable task and
returns at once:
task = SolidObjects.dead_letters.effects.redrive(
actor_type: "payments", failed_after: 6.hours.ago, limit: 5_000,
authorization_context: current_admin
)
task.cancel(authorization_context: current_admin)
The task is idempotent over its scope and filters, which a dashboard
button needs. A unique index on the active scope enforces that in the
database rather than in a read followed by a write, so two processes
that start the same redrive at the same time share one task. The index
is total rather than partial: the column holds the scope while the task
runs and NULL once it finishes, so a later redrive of the same scope
starts a new task.
The supervisor advances one bounded batch per pass and pauses between
batches, so a redrive of thousands of rows never holds a transaction
longer than one batch and shares the database with delivery.
`SolidObjects.redrives` reads tasks back by id and by status. A running
task reports what is left to move rather than a stored estimate, because
rows die and are retried while it runs. Every transition writes one
audit row under the identity that asked for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The roadmap still named the gaps this branch closes: no retry for a dead effect or broadcast, one row at a time, and no record of who pressed what. It now states what exists and what still does not, which is the dashboard surface for the new scopes. Operations gains the API, the idempotency rule, the batching, the authorization resource names, and the audit row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This comment has been minimized.
This comment has been minimized.
A redrive read the scope on every pass, so a row it moved that failed again landed straight back in it. With a handler that was still broken, a task without a limit would move the same rows forever and never finish. Porting this to the TypeScript runtime is where it showed: there the workers run beside the redrive, so the churn was immediate. A pass now takes only rows that were already dead when the task started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RedriveManager#start` is public and ran no check of its own, so the only guard was the scope that normally calls it. A caller that reached the manager directly started a task unauthorized. The check moves into `start`, under the scope's own resource name, so there is one check and no way around it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Redrive.lock` takes a plain `FOR UPDATE`, so with a redrive thread in every supervisor the second one waits on the first rather than taking the next task. It uses `lock_candidates` now, which is `FOR UPDATE SKIP LOCKED` where the database has it, as the actor, effect, broadcast, and reminder claims already do. `RedriveTask#cancel` moves out of the `Data.define` block, because rbs-inline does not read that block and the shipped signature therefore omitted a documented method. The migrations inline their one-use JSON helper, and four comments that restated the code they sat above are gone. The reasoning they carried is in the commit that introduced each one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
Reviewing the TypeScript port surfaced two defects this branch shares. `limit` and `failed_after` were passed through unchecked. A limit of zero or a fraction reached the database as a filter nobody had agreed to, and a value that does not answer `utc` raised a NoMethodError from inside the manager rather than refusing the argument. `close` updated a task by id alone, so a cancel could overwrite a task the runner had already completed and write a second transition event for it. Both closes guard on the running status now and write their event only when the update changed a row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`active_scope_for` wrapped one interpolation and `batch_size` wrapped one calculation, each with a single caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
Two public helpers validated one argument each, with one caller and a generated signature apiece. They are guard clauses in `redrive` now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
The TypeScript suite covers this and the Ruby suite did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The audit was written before the work, in its own transaction, so a retry that raised left a record of something that never happened. A message retry that could not enqueue, and a scope retry whose revive failed, both wrote one. Each event goes in the transaction that causes it now. The same defect was in the TypeScript port and is fixed there too, which is where it surfaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
`retried_reference` wrapped one branch with one caller. `retry` shows the branch and `enqueue_retry` keeps only the enqueue, which the branch needs a name for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
An event records an authorized press, not a state transition, and the redrive transitions are the opposite. The rule was implicit in the tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptileai review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #73.
Why
DeadLetterManagercovered message dead letters only. A dead effect orbroadcast reached
status = 'dead'through the same check constraint andnothing brought it back, so the recovery path was an operator writing an
UPDATEagainst a runtime table. That is the one thing the deny-by-defaultposture exists to prevent, and for a transmit effect it meant a dead one was a
lost replay.
Retry was also one row at a time. An incident produces dead rows in the
hundreds.
The kind rides on the receiver
SolidObjects.dead_letters.alland.retry(id)behave exactly as they did.Each scope reads and retries only its own kind and authorizes under its own
resource name:
effect_dead_letters,broadcast_dead_letters,redrives.Retry
It returns the row to pending with a zero attempt count, no claim, and immediate
availability, and it keeps the stable id so a handler that deduplicates on
effect_idstill sees the same key.Two decisions worth review, because they are mine rather than the issue's.
Retry acts only on a dead row. A row that is pending, processing, or
completed comes back unchanged. That is what makes a second press a no-op, and
it also means a retry can never take a row away from a worker that holds it.
Retry takes the stable id,
effect_idorbroadcast_id, not the primarykey. The issue names the parameters that way, and it is the id a deduplicating
handler keys on. Message dead letters still take the row id, which is their
existing contract.
Redrive
Idempotency is a database constraint, not a read followed by a write. A
redrivewrites its scope and filter digest into a uniqueactive_scopecolumn, so two processes that start the same redrive at the same instant share
one task rather than race. The column holds the digest while the task runs and
NULLonce it finishes, which makes the index total rather than partial, sinceAGENTS.mdrules partial indexes out. The same scope can be redriven again oncethe first task ends.
The supervisor advances it. One bounded batch per pass, each its own short
transaction, with
redrive_batch_pausebetween batches, so a redrive ofthousands of rows never holds a transaction longer than one batch and shares the
database with delivery.
redrive_batch_sizedefaults to 100 andredrive_batch_pauseto 0.05 seconds.A running task reports what is left to move, counted at read time, rather
than a stored estimate. Rows die and are retried while a redrive runs, so a
stored number would drift.
Audit
Every retry and every redrive transition writes one row to
solid_objects_administration_events: action, kind, subject, filters, identity,and when. The identity comes from the authorization context the caller already
passes, through a new
administration_identityhook that defaults to itsto_sand is bounded to 255 bytes. A refused caller writes nothing, because the audit
records what happened rather than what was denied. A read writes nothing.
Schema
Two tables,
solid_objects_administration_eventsandsolid_objects_redrives.No change to
effects,broadcasts, ordead_letters, becausestatusalready carries
deadandpending. Both are added to the doctor's schemacheck and to the public
TestHelperreset list, which has a guard test thatfails when a table is missing from it.
Tests
39 new tests across three files, each watched failing first.
test/integration/dead_letter_scopes_test.rbcovers the issue's list: a deadeffect returns to pending and runs again, a retried effect reuses its stable id,
retrying a pending effect changes nothing, a dead broadcast returns and
delivers, a dead transmit effect replays,
SolidObjects.dead_lettersisunchanged, each scope reads only its own kind and only dead rows, and an
unauthorized caller is refused.
test/integration/redrive_test.rbcovers bounded batches, the limit, runningidempotency, a separate task per scope and per filter set, a new task after the
first finishes, cancel leaving moved rows moved, both filters, reading tasks
back, one audit row per transition, refusal, the frozen value object, and a
supervised runtime draining a redrive with no caller driving it.
test/integration/administration_audit_test.rbcovers one row per press for allthree kinds, the configured identity, and no row for a refusal or a read.
The behaviours are load-bearing, not just the method names. Removing the batch
cap failed three tests; making the scope digest random failed the idempotency
test with two rows where one was expected.
Validation
bundle exec rakepasses, including Standard, RuboCop, RBS, Steep, andBrakeman.
What this does not do
Automatic redrive on a schedule, which the issue puts out of scope.
The dashboard does not yet surface the new scopes or redrive. The issue
specifies an API and an audit trail, not a UI, and the roadmap entry now says so
rather than implying the dashboard covers it.
🤖 Generated with Claude Code